Skip to content

Conversation

gorriecoe
Copy link

Hi @adoyle-h,

Great idea for a package. I have made a couple of changes that I think will make a bit more reliable.

Added string-template for formating.   Refer to https://www.npmjs.com/package/string-template
Removed L file.  Now calls logic file directly.
Updated documentation to suite.
@adoyle-h
Copy link
Owner

Hi @gorriecoe ,

These changes are reasonable. But there is something wrong with code style. Please change it and make sure all code style checking and test cases passed in CI. I will merge it and publish at version 1.0.0.

Thanks for your contributions.

@gorriecoe
Copy link
Author

Hi @adoyle-h,

I have fixed all errors except your last test.

@adoyle-h
Copy link
Owner

test('throw error when no matched variable ', (t) => {
    const error = t.throws(() => logic('!{a}', {ha: false}));
    t.is(error instanceof Error, true);
    t.is(error.message, 'No any matched variable for "a"');
});

I expect to throw an error when no matched variable in the string template.
string-template('!{a}', {ha: false})) will return "!", and then eval("!") will cause the "Unexpected end of input" error.
I think missing variable is unstable and dangerous. It is better to throw error immediately at this line.

I suggest you copy and modify string-template codes (with its copyright notice) into the logic-string repo for full control. Or make a PR to the string-template repo.

@gorriecoe

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants